Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minilcm diff API #1181

Merged
merged 43 commits into from
Nov 14, 2024
Merged

Minilcm diff API #1181

merged 43 commits into from
Nov 14, 2024

Conversation

hahn-kev
Copy link
Collaborator

@hahn-kev hahn-kev commented Oct 30, 2024

related to #1127

  • uses the new harmony version to put a commit id on each object
  • creates a new UpdateEntry(Entry entry) api for updating using the version to ensure we don't stomp on changes
  • write some fuzzing tests using randomly generated entries for create and update tests

hahn-kev and others added 25 commits October 17, 2024 14:54
was current/previous now before after
# Conflicts:
#	backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateSenseProxy.cs
#	backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs
#	backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs
#	backend/FwLite/MiniLcm/Models/Entry.cs
# Conflicts:
#	.idea/.idea.LexBox/.idea/dataSources.xml
#	backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs
Copy link

github-actions bot commented Oct 30, 2024

C# Unit Tests

75 tests  ±0   75 ✅ ±0   5s ⏱️ ±0s
13 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 9a315be. ± Comparison against base commit 85f9555.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@myieye myieye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a bit of progress

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs Outdated Show resolved Hide resolved
backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs Outdated Show resolved Hide resolved
backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs Outdated Show resolved Hide resolved
backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs Outdated Show resolved Hide resolved
backend/FwLite/MiniLcm/SyncHelpers/SenseSync.cs Outdated Show resolved Hide resolved
backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs Outdated Show resolved Hide resolved
backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs Outdated Show resolved Hide resolved
@hahn-kev
Copy link
Collaborator Author

hahn-kev commented Nov 4, 2024

had a chat with Tim, and we realized this approach won't work using version numbers and such, otherwise since updates could come from before that version was made, we can't reliably get that version, if we didn't then we could overwrite someone elses changes.

# Conflicts:
#	backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
#	backend/FwLite/FwLiteProjectSync.Tests/Fixtures/SyncFixture.cs
#	backend/FwLite/FwLiteProjectSync.Tests/UpdateDiffTests.cs
#	backend/FwLite/FwLiteProjectSync/CrdtFwdataProjectSyncService.cs
#	backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs
#	backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs
#	backend/FwLite/LcmCrdt/LcmCrdtKernel.cs
#	backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs
#	backend/FwLite/MiniLcm/InMemoryApi.cs
#	backend/FwLite/MiniLcm/SyncHelpers/EntrySync.cs
…hen an entry is created, fix issue with ordering of adding complex forms to an entry
@hahn-kev hahn-kev marked this pull request as ready for review November 12, 2024 02:57
@hahn-kev hahn-kev requested a review from myieye November 12, 2024 02:57
Copy link
Contributor

@myieye myieye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.
The commit removing object versions was a bit sad to look at 😢

@hahn-kev hahn-kev merged commit b188a5d into develop Nov 14, 2024
16 checks passed
@hahn-kev hahn-kev deleted the minilcm-diff-apis branch November 14, 2024 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants